Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Story for AddressMotionToVacateView #14777

Merged
merged 5 commits into from
Jul 31, 2020

Conversation

jcq
Copy link
Contributor

@jcq jcq commented Jul 27, 2020

Description

This adds a story for AddressMotionToVacateView component as a way to demonstrate how to use decorators to wrap stories when components depend on things like react-router or redux.

Acceptance Criteria

  • Code compiles correctly
  • Story for AddressMotionToVacateView renders properly

(shows how to use decorators for router & redux)

const Template = () => <AddressMotionToVacateView />;

export const Basic = Template.bind({});
Copy link
Contributor Author

@jcq jcq Jul 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (setting up a template once and then basing all stories off of it) is the recommended syntax for stories in Storybook 6.x. While it isn't particularly useful in a case with only a single story, I used it here for example purposes.

title:
'Queue/Motions to Vacate/Judge Address Motion to Vacate/AddressMotionToVacateView',
component: AddressMotionToVacateView,
decorators: [RouterDecorator, ReduxDecorator],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can specify as many decorators as you want. Each will simply wrap the next.

@codeclimate
Copy link

codeclimate bot commented Jul 27, 2020

Code Climate has analyzed commit e3ccc16 and detected 0 issues on this pull request.

View more on Code Climate.

};

const RouterDecorator = (storyFn) => (
<StaticRouter
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a chance that using MemoryRouter here might actually be better; StaticRouter works fine here since we have only one story and don't use react-router for anything other than grabbing the matched params.

appealDetails: {
[appealId]: appeal,
},
amaTasks: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept this seed data to the bare minimum necessary to render the component in the story. This should properly contain the whole task tree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be some seed data missing, I'm getting an error for the partial grant option (will upload a screenshot on the PR).

@leikkisa
Copy link
Contributor

@jcq This mostly looks great! I'm getting this error when I select the partial grant option.
Screen Shot 2020-07-27 at 7 11 17 PM

@jcq
Copy link
Contributor Author

jcq commented Jul 29, 2020

I'm getting this error when I select the partial grant option.

@leikkisa Updated with the additional seed data necessary to avoid that issue!

@va-bot
Copy link
Collaborator

va-bot commented Jul 29, 2020

1 Warning
⚠️ This PR modifies React components — consider adding/updating corresponding Storybook file

Generated by 🚫 Danger

Copy link
Contributor

@leikkisa leikkisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jcq jcq added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Jul 30, 2020
@va-bot va-bot merged commit 1d55f63 into master Jul 31, 2020
@va-bot va-bot deleted the jc/story-AddressMotionToVacateView branch July 31, 2020 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Eng: Frontend Work Group frontend engineering task Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master Team: Foxtrot 🦊
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants